-
Notifications
You must be signed in to change notification settings - Fork 803
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid erroring for source distributions with symlinks in archive #1944
Conversation
9ed01db
to
c2fecaa
Compare
I need to test this with a symlinked directory before merging. |
crates/uv-extract/src/stream.rs
Outdated
|
||
// On Windows, skip symlink entries, as they're not supported. pip recursively copies the | ||
// symlink target instead. | ||
#[cfg(windows)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make this if cfg!(windows)
so the branch gets checked when compiling on unix?
dbad0b0
to
3a07005
Compare
Okay, I mean, weird things happen if you include a symlink directory. On macOS, if you run create a symlink directory in |
Summary
For context, we have three extraction paths:
.tar.gz
, local or remote.We use three different crates for these:
tokio-tar
async-zip
zip-rs
These all seem to have different support for symlinks:
tokio-tar
tries to create a symlink (which works fine on Unix but errors on Windows, since we typically don't have elevated permissions).async-zip
seems to write the target contents directly to the file (which is what we want).zip-rs
apparently writes the name of the target to the file (which isn't what we want).Thankfully, symlinks are not allowed in wheels (pypa/pip#5919, https://discuss.python.org/t/symbolic-links-in-wheels/1945), so we can ignore
zip-rs
.For
tokio-tar
, we now skip (and warn) if we see a symlink on Windows. We could do what pip does, and recursively copy, but it's difficult because we don't haveSeek
on the file. (Alternatively, we could use hard links and junctions, though those also might need to exist already.) Let's see how far this gets us.(We also no longer attempt to set permissions on symlinks on Unix, which caused another failure.)
Closes #1858.